New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add hamburger menu on small screens #16388
Conversation
@@ -105,17 +105,26 @@ Polymer({ | |||
|
|||
/** @private */ | |||
onMenuTap_: function() { | |||
console.debug('[br_toolbar] Not Implemented: onMenuTap_') | |||
this.dispatchEvent(new CustomEvent( | |||
'cr-toolbar-menu-tap', {bubbles: true, composed: true})) | |||
}, | |||
|
|||
focusMenuButton() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if these (other than onMenuTap_
) are needed but I thought I might as well implement them while I'm adding the button.
@petemill mind taking a look at this? |
A Storybook has been deployed to preview UI for the latest push |
Oh wow, that's funky. I'll rebase and see if it repros |
Wow, I've been digging into this a bit more. It seems to be caused by some crazy interplay between the polymer |
Looks like it's probably caused by this update |
bb62276
to
98e70d5
Compare
A Storybook has been deployed to preview UI for the latest push |
A Storybook has been deployed to preview UI for the latest push |
@simonhong looks like the latest Chromium version adds proper support for |
Sorry, I couldn't give a good advice for this PR. I'll pass review to @petemill |
Nah, no worries 😆 I have no idea what caused the weird breaks in the sidebar. Thanks for all your help! |
58d087e
to
c46ff13
Compare
c46ff13
to
a453eac
Compare
A Storybook has been deployed to preview UI for the latest push |
bfce596
to
8f679b8
Compare
8f679b8
to
485629f
Compare
5a98938
to
f8a6aee
Compare
16d2e5c
to
fad603a
Compare
fad603a
to
f30ef99
Compare
584a74e
to
d6dbe17
Compare
@simonhong mind taking another look, now everything under it has merged? |
A Storybook has been deployed to preview UI for the latest push |
Exciting! This might finally be ready 😆 @petemill & @simonhong mind having another review? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
++ works great on linux also :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Beautiful
Resolves brave/brave-browser#24352
Screencast.from.2022-12-16.14-52-57.webm
Note: This depends on #18156 which enables using Leo from inside Polymer.
Note: The color icons don't look right at the moment because there's a bug in the Leo Icon component - the fix is here brave/leo#269
Submitter Checklist:
QA/Yes
orQA/No
;release-notes/include
orrelease-notes/exclude
;OS/...
) to the associated issuenpm run test -- brave_browser_tests
,npm run test -- brave_unit_tests
wikinpm run lint
,npm run presubmit
wiki,npm run gn_check
,npm run tslint
git rebase master
(if needed)Reviewer Checklist:
gn
After-merge Checklist:
changes has landed on
Test Plan: